Fix compiler warnings - #255
Merged
Merged
Conversation
It is not appropriate to declare that a function exists, when we very well know that it does in fact not exist (anymore). That was the approach taken in dgutov#254. If we revert that, we are back to: diff-hl.el:1677:19: Warning: the function ‘project-buffers’ is not known to be define diff-hl.el:1668:9: Warning: the function ‘project-roots’ is not known to be defined. diff-hl.el:1654:17: Warning: the function ‘project-name’ is not known to be defined. Once we add: (eval-when-compile (require 'project)) we are left with diff-hl.el:414:16: Warning: ‘revert-buffer-in-progress-p’ is an obsolete variable (as of 31.1); use ‘revert-buffer-in-progress’ instead. which we deal with using `static-if'. Unfortunately that macro was only added in Emacs 30.1. To get our hands on it earlier, we have two options. Either we start depending on the Compat package, which provides this macro, or we define it ourselves. I went with the latter approach, assuming that you wouldn't want to add an additional dependency just for this. But surely it is not okay to (re-)define a macro that is usually defined in (later versions of) Emacs?! Glad you asked! The implementation in "subr.el" is prefixed with: ;; Note: `static-if' can be copied into a package to enable it to be ;; used in Emacsen older than Emacs 30.1. If the package is used in ;; very old Emacsen or XEmacs (in which `eval' takes exactly one ;; argument) the copy will need amending.
The macro `diff-hl-with-diff-switches' is define and used in "diff-hl.el". Additionally it is used in "diff-hl-flydiff.el". This macro uses variables from the `vc' package without requiring it up front. Until now this was dealt with using top-level variable declarations in "diff-hl.el". That is not enough because the expansion of that macro, in another file, does not benefit from these declarations. diff-hl-flydiff.el:47:8: Warning: Unused lexical variable ‘vc-svn-diff-switches’ diff-hl-flydiff.el:47:8: Warning: Unused lexical variable ‘vc-fossil-diff-switches’ diff-hl-flydiff.el:47:8: Warning: Unused lexical variable ‘vc-jj-diff-switches’ So we have to add the variable declarations to the macro expansion themselves.
Contributor
|
Thanks for the detailed explanation, Jonas. Sorry for my naive approach. |
Owner
|
Thanks Jonas! That seems to cover all the bases, aside from the minor issue of project.el being an ELPA package - the user could upgrade to a later version. For example, from one that only has Maybe not that big a deal, though, and it might be time to drop 26/27 support sometime soon anyway. |
Contributor
Author
|
Ah, I didn't realize (or (and (fboundp 'project-root) (project-root proj))
(and (fboundp 'project-roots) (project-roots proj))) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've implemented these fixes several weeks ago but did not submit them because of the work involved justifying the changes. I am now submitting them anyway because another attempt to fix1 some of these warnings was just merged, and that approach was wrong.
See the individual commit for more information.
Footnotes
fix(diff-hl.el): Silence compiler warnings #254 /cc @bcardoso ↩